Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support configurable branding #2589

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

llewelld
Copy link
Contributor

These changes essentially allow the branding of the installer to be configured by editing the "branding.json" file.

From the user's perspective, behaviour should be unchanged after these changes: the included "branding.json" file has been configured to match the existing behaviour.

The ubports-installer is a really nice approach, kudos to your amazing development here. The background is that I was hoping it would be possible to use it for Sailfish OS as well. This is a personal initiative currently, but if it's going to be taken up as the main Sailfish installation method, it would need to be branded differently (changes to some strings and graphics). The purpose of this PR is to minimise and centralise the changes needed to allow this, so that the UBports installer is unaffected.

I can understand that you may not want to integrate this into your code, but if it looks viable, I'm of course also very open to suggestions for how to improve on the approach. I'm not a svelte/nodejs developer, so this is all somewhat new to me.

Ultimately, my plan would be to create a separate repo with the ubports-installer as a submodule, and to patch it with a Sailfish-oriented branding file.

@llewelld
Copy link
Contributor Author

Force pushed a change to fix the unit test failure introduced by my earlier commits.

@codecov
Copy link

codecov bot commented Apr 22, 2022

Codecov Report

Merging #2589 (4601f26) into master (9a39df2) will increase coverage by 0.22%.
The diff coverage is 88.88%.

@@            Coverage Diff             @@
##           master    #2589      +/-   ##
==========================================
+ Coverage   76.20%   76.42%   +0.22%     
==========================================
  Files          29       29              
  Lines         954      963       +9     
==========================================
+ Hits          727      736       +9     
  Misses        227      227              
Impacted Files Coverage Δ
src/lib/mainEvent.js 35.78% <50.00%> (+0.68%) ⬆️
src/main.js 38.00% <50.00%> (+1.26%) ⬆️
src/core/helpers/api.js 100.00% <100.00%> (ø)
src/lib/cli.js 100.00% <100.00%> (ø)
src/lib/log.js 100.00% <100.00%> (ø)
src/lib/menuManager.js 100.00% <100.00%> (ø)
src/lib/reporter.js 100.00% <100.00%> (ø)
src/lib/udev.js 100.00% <100.00%> (ø)
src/lib/updater.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9a39df2...4601f26. Read the comment docs.

@llewelld
Copy link
Contributor Author

Force pushed changes to allow the package building check step to now (hopefully) pass.

@llewelld
Copy link
Contributor Author

I pushed some further small changes (which again, shouldn't affect the default configuration) following more testing.

Allow the overall branding (app name, urls, etc.) to be controlled by
editing values in the branding.json file.
Allows the image and icon folders to be set as part of the branding
configuration.
Ensures the supported-devices link appears only if it's set.

Tweaks the text on the Done page to avoid confusion in case non-Ubuntu
Touch branding is being used.
@llewelld
Copy link
Contributor Author

Updated to revert changes to the Copyright line.

@NeoTheThird
Copy link
Member

Thanks for the interesting proof-of-concept! I have been thinking about this a little a while back. Could have sworn there was an issue with some notes somewhere, but i can't find it right now. Basically, yes i want this for the UBports Installer! It needs to support all the operating systems as first-class citizens, and that means that all the hardcoded mentions of Ubuntu Touch need to disappear. I am however a little bit on the fence when it comes to the idea of having "downstream" builds with their own bugtrackers and possible patches diverging heavily from upstream. Dunno. Feels like we'd be creating a bunch of duplicate effort... Instead of creating forks of the installer, it might make more sense to add OS-specific CSS that gets applied during the installation. Like, an Ubuntu Touch orange-and-purple theme gets loaded if Ubuntu Touch is selected, a SailfishOS theme gets applied if SailfishOS is selected, etc. We can also add more options to customize donation- and support links in the device configs.

Sure, it the software would still be called UBports Installer, but that is just the name of the organization that has primarily funded the development of the installer. Note that it is not the "Ubuntu Touch Installer" :) If that name is preventing other projects from recommending it, then i would be open to discussing a name change. But every project creating a downstream with the name of their OS stamped on it... I don't know if that's the greatest outcome. Would go a little bit against the idea of projects coming together to create the installer, imho. But maybe that's just my opinion.

@sanathusk
Copy link

+1. Please let me know you need any help with this.i would be interested in contributing to the efforts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants